some cleanup #840

winfriedgerlach · 2025-07-07T15:21:09Z

fixed lots of small but minor issues, hope you find this helpful!

winfriedgerlach · 2025-07-07T15:22:14Z

.gitignore

@@ -124,7 +124,6 @@ local.properties
 .factorypath
 .project
 .settings
-.springBeans


duplicate of line 121

robogary · 2025-07-10T11:12:24Z

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

jkiddo · 2025-07-10T11:14:37Z

@winfriedgerlach - please run mvn spotless:apply to fix the formatting violations.

jkiddo · 2025-07-10T11:15:22Z

Besides that, LGTM

winfriedgerlach · 2025-07-11T13:22:56Z

hey @jkiddo , sorry, fixed now! I ran mvn spotless:apply and pushed the changes

robogary · 2025-07-11T13:36:10Z

Formatting check succeeded!

jkiddo · 2025-08-01T06:52:53Z

Looks good to me - I'd like a second set of 👀 on it as well

Copilot

Pull Request Overview

This PR focuses on cleaning up code quality issues across the codebase by standardizing syntax, fixing spelling errors, and applying modern Java patterns. The changes improve maintainability and consistency without altering functionality.

Updated variable declarations from var to let in JavaScript and deprecated Java patterns to modern alternatives
Corrected numerous spelling and grammatical errors in comments, documentation, and configuration files
Applied modern Java language features like pattern matching with instanceof and text blocks

Reviewed Changes

Copilot reviewed 32 out of 33 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/test/smoketest/plain_server.http	Updated variable declarations from `var` to `let`
src/test/smoketest/SMOKE_TEST.md	Fixed spelling errors in documentation
src/test/resources/application.yaml	Corrected comment syntax and spelling errors
src/test/java/some/custom/pkg1/CustomInterceptorPojo.java	Applied pattern matching with instanceof
src/test/java/some/custom/pkg1/CustomInterceptorBean.java	Applied pattern matching with instanceof
src/test/java/ca/uhn/fhir/jpa/starter/SocketImplementation.java	Removed printStackTrace call and improved logging
src/test/java/ca/uhn/fhir/jpa/starter/ParallelUpdatesVersionConflictTest.java	Modernized lambda expressions and removed unused imports
src/test/java/ca/uhn/fhir/jpa/starter/MeasureEvaluationConfig.java	Removed unused imports
src/test/java/ca/uhn/fhir/jpa/starter/IServerSupport.java	Replaced deprecated Charsets with StandardCharsets
src/test/java/ca/uhn/fhir/jpa/starter/ExampleServerR4IT.java	Replaced string concatenation with text blocks and improved assertions
src/test/java/ca/uhn/fhir/jpa/starter/ExampleServerR4BIT.java	Replaced string concatenation with text blocks
src/test/java/ca/uhn/fhir/jpa/starter/ExampleServerDstu3IT.java	Removed unnecessary blank lines
src/test/java/ca/uhn/fhir/jpa/starter/ExampleServerDbpmR5IT.java	Changed class visibility modifier
src/test/java/ca/uhn/fhir/jpa/starter/ElasticsearchLastNR4IT.java	Removed unused imports and changed class visibility
src/test/java/ca/uhn/fhir/jpa/starter/CdsHooksServletIT.java	Replaced string concatenation with text blocks and fixed spelling
src/main/webapp/WEB-INF/templates/about.html	Updated variable declarations and fixed punctuation
src/main/resources/cds.application.yaml	Fixed spelling errors and comment syntax
src/main/resources/application.yaml	Fixed spelling errors and comment syntax
src/main/java/ca/uhn/fhir/jpa/starter/util/EnvironmentHelper.java	Applied pattern matching with instanceof
src/main/java/ca/uhn/fhir/jpa/starter/cr/PostInitProviderRegisterer.java	Made inner class static
src/main/java/ca/uhn/fhir/jpa/starter/common/validation/RepositoryValidationInterceptorFactoryR5.java	Fixed variable naming consistency
src/main/java/ca/uhn/fhir/jpa/starter/common/validation/RepositoryValidationInterceptorFactoryR4B.java	Fixed variable naming consistency
src/main/java/ca/uhn/fhir/jpa/starter/common/validation/RepositoryValidationInterceptorFactoryR4.java	Fixed variable naming consistency
src/main/java/ca/uhn/fhir/jpa/starter/common/StarterJpaConfig.java	Improved logging and simplified Optional usage
src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java	Modernized logging with parameterized messages
src/main/java/ca/uhn/fhir/jpa/starter/cdshooks/ModuleConfigurationPrefetchSvc.java	Simplified collection size check
src/main/java/ca/uhn/fhir/jpa/starter/cdshooks/CdsHooksServlet.java	Added @serial annotation
src/main/java/ca/uhn/fhir/jpa/starter/Application.java	Fixed spelling in comment
pom.xml	Fixed comment punctuation
charts/hapi-fhir-jpaserver/README.md.gotmpl	Fixed spelling error
charts/hapi-fhir-jpaserver/README.md	Fixed table formatting and spelling error
README.md	Fixed spelling errors throughout documentation

Copilot · 2025-08-01T06:54:02Z

src/test/resources/application.yaml

@@ -31,7 +31,7 @@ spring:
    allow-bean-definition-overriding: true
  flyway:
    enabled: false
-    check-location: false
+    fail-on-missing-locations: false


The property name change from check-location to fail-on-missing-locations appears to be changing the Spring Flyway configuration property. Verify that this property name is correct for the version of Spring Boot being used, as this could cause configuration to be ignored.

Suggested change

fail-on-missing-locations: false

check-location: false

AFAIU this is the non-deprecated replacement of "check-location"

Copilot · 2025-08-01T06:54:03Z

src/main/resources/cds.application.yaml

+    # or will be instantiated via reflection using a no-arg contructor; then registered with the server
    #custom-interceptor-classes:  

    # comma-separated list of fully qualified provider classes. 
    # classes listed here will be fetched from the Spring context when combined with 'custom-bean-packages', 
-    # or will be instantiated via reflection using an no-arg contructor; then registered with the server  
+    # or will be instantiated via reflection using a no-arg contructor; then registered with the server


The word 'contructor' should be spelled 'constructor'.

Copilot · 2025-08-01T06:54:03Z

src/main/resources/cds.application.yaml

    #custom-interceptor-classes:  

    # comma-separated list of fully qualified provider classes. 
    # classes listed here will be fetched from the Spring context when combined with 'custom-bean-packages', 
-    # or will be instantiated via reflection using an no-arg contructor; then registered with the server  
+    # or will be instantiated via reflection using a no-arg contructor; then registered with the server


The word 'contructor' should be spelled 'constructor'.

Copilot · 2025-08-01T06:54:04Z

src/main/resources/application.yaml

@@ -269,12 +269,12 @@ hapi:

    # comma-separated list of fully qualified interceptor classes. 
    # classes listed here will be fetched from the Spring context when combined with 'custom-bean-packages', 
-    # or will be instantiated via reflection using an no-arg contructor; then registered with the server  
+    # or will be instantiated via reflection using a no-arg contructor; then registered with the server


The word 'contructor' should be spelled 'constructor'.

Suggested change

# or will be instantiated via reflection using a no-arg contructor; then registered with the server

# or will be instantiated via reflection using a no-arg constructor; then registered with the server

Copilot · 2025-08-01T06:54:04Z

src/main/resources/application.yaml

    #custom-interceptor-classes:  

    # comma-separated list of fully qualified provider classes. 
    # classes listed here will be fetched from the Spring context when combined with 'custom-bean-packages', 
-    # or will be instantiated via reflection using an no-arg contructor; then registered with the server  
+    # or will be instantiated via reflection using a no-arg contructor; then registered with the server


The word 'contructor' should be spelled 'constructor'.

winfriedgerlach · 2025-08-08T08:44:55Z

@jkiddo @patrick-werner fixed spelling mistakes found by CoPilot

robogary · 2025-08-10T16:54:41Z

Formatting check succeeded!

jkiddo · 2025-08-16T13:56:39Z

@winfriedgerlach did you check up on the error that is now present? https://github.com/hapifhir/hapi-fhir-jpaserver-starter/actions/runs/16826070940/job/47792357339?pr=840#step:4:4203

winfriedgerlach · 2025-08-18T08:38:35Z

@winfriedgerlach did you check up on the error that is now present? https://github.com/hapifhir/hapi-fhir-jpaserver-starter/actions/runs/16826070940/job/47792357339?pr=840#step:4:4203

@jkiddo Oops, that's really weird. I could reproduce the issue locally and just reverted my change for now.

robogary · 2025-08-18T20:24:51Z

Formatting check succeeded!

jkiddo · 2025-08-18T20:38:44Z

@winfriedgerlach one linting error left and we can merge this

winfriedgerlach · 2025-08-19T07:05:07Z

@jkiddo wow, the linter requires unreadable table formatting? :-D
I reverted my change, hope everything is fine now

robogary · 2025-08-19T07:07:03Z

Formatting check succeeded!

jkiddo · 2025-08-19T07:10:22Z

@chgl you good with the linting errors in the charts readme?

winfriedgerlach · 2025-08-19T07:13:01Z

hmm, same version 0.20.0 is on master...

chgl · 2025-08-19T07:41:38Z

hmm, same version 0.20.0 is on master...

If something in the charts/* folder changes, the version of the chart.yaml also needs to be bumped. Even if it's just cosmetics in the readme. So you can change https://github.com/hapifhir/hapi-fhir-jpaserver-starter/blob/master/charts/hapi-fhir-jpaserver/Chart.yaml#L17 to 0.20.1 and it should work. (Ideally also change the changelog annotations under https://github.com/hapifhir/hapi-fhir-jpaserver-starter/blob/master/charts/hapi-fhir-jpaserver/Chart.yaml#L26, but not strictly necessary).

winfriedgerlach · 2025-08-19T09:22:57Z

@chgl like this?

robogary · 2025-08-19T09:37:19Z

Formatting check succeeded!

chgl · 2025-08-19T11:34:42Z

@winfriedgerlach perfect, thank you!

patrick-werner · 2025-08-11T15:05:22Z

src/test/java/ca/uhn/fhir/jpa/starter/CdsHooksServletIT.java

+			for (var service : services) {
+				myCdsServiceRegistry.unregisterService(service.getId(), "CR");


This causes a java.util.ConcurrentModificationException, either leave the index based iteration, or create a copy:

Suggested change

for (var service : services) {

myCdsServiceRegistry.unregisterService(service.getId(), "CR");

for (var service : new ArrayList<>(services)) {

myCdsServiceRegistry.unregisterService(service.getId(), "CR");

}

my only issue was this error, but this change was reverted in the meantime

some cleanup

2d833fc

winfriedgerlach commented Jul 7, 2025

View reviewed changes

results from mvn spotless:apply

db4d427

jkiddo requested review from patrick-werner and Copilot August 1, 2025 06:52

Copilot AI reviewed Aug 1, 2025

View reviewed changes

fix contructor --> constructor

81d0512

revert change to fix CdsHooksServletIT

0f9e834

revert change to charts README.md

93247fa

bump chart version, required due to changes in README.md

40e08cf

patrick-werner approved these changes Aug 19, 2025

View reviewed changes

jkiddo merged commit 8224cae into hapifhir:master Aug 19, 2025
17 of 21 checks passed

	# or will be instantiated via reflection using a no-arg contructor; then registered with the server
	# or will be instantiated via reflection using a no-arg constructor; then registered with the server

		for (var service : services) {
		myCdsServiceRegistry.unregisterService(service.getId(), "CR");

some cleanup #840

some cleanup #840

Uh oh!

Conversation

winfriedgerlach commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robogary commented Jul 10, 2025

Uh oh!

jkiddo commented Jul 10, 2025

Uh oh!

jkiddo commented Jul 10, 2025

Uh oh!

winfriedgerlach commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robogary commented Jul 11, 2025

Uh oh!

jkiddo commented Aug 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

winfriedgerlach commented Aug 8, 2025

Uh oh!

robogary commented Aug 10, 2025

Uh oh!

jkiddo commented Aug 16, 2025

Uh oh!

winfriedgerlach commented Aug 18, 2025

Uh oh!

robogary commented Aug 18, 2025

Uh oh!

jkiddo commented Aug 18, 2025

Uh oh!

winfriedgerlach commented Aug 19, 2025

Uh oh!

robogary commented Aug 19, 2025

Uh oh!

jkiddo commented Aug 19, 2025

Uh oh!

winfriedgerlach commented Aug 19, 2025

Uh oh!

chgl commented Aug 19, 2025

Uh oh!

winfriedgerlach commented Aug 19, 2025

Uh oh!

robogary commented Aug 19, 2025

Uh oh!

chgl commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

winfriedgerlach commented Jul 11, 2025 •

edited

Loading